Enable MD workflows for any ASE calculator#39
Enable MD workflows for any ASE calculator#39steinmig wants to merge 6 commits intolearningmatter-mit:masterfrom
Conversation
steinmig
commented
Aug 26, 2025
- some small typing corrections and cleanups
HojeChun
left a comment
There was a problem hiding this comment.
If I understand correctly, I like your idea of unifying various calculators. But, I have some concerns you might want to consider.
Thanks!
There was a problem hiding this comment.
I am not sure why you made changes for this. if you need load_foundations_path function as class methods, I guess you keep the original function and make load_foundations_path and make something like.
@classmethod
def load_foundations_path (cls, model, map_location, default_dtype):
mace_model_path = get_mace_mp_model_path(model)
return cls.load_foundations_path(mace_model_path, map_location, default_dtype)
There was a problem hiding this comment.
I don't quite understand what you mean. As far as I understand your comment, this is what I did
There was a problem hiding this comment.
i mean get_mace_mp_model_path is basically load_foundations_path. If your intention was to have a classmethod of getting model_path I would decouple the functions as
@classmethod
def load_foundations_path (cls, model, map_location, default_dtype):
mace_model_path = get_mace_mp_model_path(model)
return mace_model_path
Sorry for a wild example above
There was a problem hiding this comment.
I like this idea, and actually I have implemented this kind of thing for my vssr calculator.
| raise RuntimeError("Batch is missing 'nxyz' key.") | ||
| pbc = batch.get("pbc") | ||
| if pbc is not None: | ||
| pbc = np.array(pbc, dtype=bool) |
There was a problem hiding this comment.
Is pbc going to be size (3,)?
There was a problem hiding this comment.
Yes exactly. It's basically the pbc member of ase.Atoms
xiaochendu
left a comment
There was a problem hiding this comment.
Looks good to me. I only have a single comment regarding the size of pbc in AsePotential